Conversation
…n_admits receiver_chain_admits() fast-admits any lower-case-rooted receiver chain as a value, not a type, which is true in Go/Python/Swift but not for C#'s built-in type aliases (int, string, bool, ...): those are lower-case KEYWORDS that name a type, so a static call like int.TryParse(...) skips the chain-consistency check and can fabricate a CALLS edge to any unrelated same-named method (DeusData#2121). Add a closed keyword set consulted only for a lower-case root, so every other language's lower-case value roots (vm.load, http.Get, os.path.join) are unaffected. Signed-off-by: Amir Fathi <amirfathi.me@gmail.com>
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
|
Both red legs here ( |
DeusData
added
bug
|
Thank you for the C# keyword fix and negative controls. The current registry's lower-case receiver handling supports the reported gap. Review should preserve the C# keyword cases without admitting unrelated receiver names in other languages. We still need to attribute the Windows failure from its evidence; an unchanged Windows-specific file list alone would not prove it unrelated. The contribution is queued for that focused review. |
|
Read the job log rather than the file list this time. The windows-latest CLANG64 leg fails inside tests/test_parallel_harness_contract.sh's own scheduler self-test, step 0i: "FAIL: Windows timeout race refused without a surviving descendant to refuse over, the fixture no longer exercises the race" (job 103034332390). That's a timing fixture for the test harness's own process cleanup, nothing in registry.c or test_pipeline.c. shard-completeness and ci-ok both fail downstream of that missing shard manifest (job 103051948933 shows no manifest uploaded for windows-latest-CLANG64-0). |
2 participants
What does this PR do?
receiver_chain_admits()(src/pipeline/registry.c) fast-admits a lower-case-rooted receiver chain as a value rather than a type. That's correct for Go, Python and Swift (vm.load,http.Get,os.path.join), but C# spells its built-in types as lower-case keywords (int,string,bool, ...), so a static call likeint.TryParse(...)skips the chain-consistency check entirely and can fabricate aCALLSedge to any unrelated method sharing the same name (#2121).Fix: a closed set of C# type keywords is checked only when the receiver root is lower-case; anything not in that set still fast-admits exactly as before, so other languages' lower-case value roots are unaffected.
Fixes #2121
What does this PR do NOT fix?
The issue's second example,
Options.Create(new object()), no longer reproduces at HEAD: PR #1897 (merged 2026-09-04) already added the chain-consistency guard this PR extends, and it happens to cover that upper-case-rooted shape already.How was this verified?
registry_receiver_chain_refuses_lowercase_type_keyword_issue2121. Without the fix,ASSERT_NULLin that test trips because the fabricated edge still resolves; with the fix it's clean. Ran both ways in the same Docker image (ubuntu:24.04, ASan+UBSan test build).index_repository+trace_path):int.TryParsefabricates an edge to an unrelatedTryParseon main; zero edges on this branch.registry_receiver_chain_keeps_type_keyword_when_chain_matches_issue2121, confirming the guard still resolves a real project type whose chain matches, so it isn't just refusing everything lower-case.pipelinesuite (279 tests) green on this branch;cppcheckandclang-format --dry-runclean on the changed file. Not checked against the exact CI toolchain: local cppcheck was 2.13.0 and clang-format 18.1.3, CI pins 2.20.0 and clang-format-20.